Fix documentation regarding suggestion for expanding proc macros #1341
Conversation
fee1-dead
force-pushed
the
suggested-config
branch
from
490c9ef to
54ffd4e
Compare
|
I'd guess we may want someone from t-libs to weigh in, if they are not interested in this suggestion maybe we can explicitly suggest one path for t-libs contributors and this one for t-compiler contributors. |
I think we could also add a note about this for those who want to contribute to the libraries rather than the compiler. (mistakenly closed, sorry!) |
JohnTitor
added
the
S-waiting-on-author
|
Should we merge this or what is it waiting on? |
|
This has a merge conflict and I forgot to ping, @fee1-dead Could you resolve it and address #1341 (comment)? |
|
From what I've seen in the source code of R-A, the current settings on the dev-guide are correct. We could just probably remove |
fee1-dead
force-pushed
the
suggested-config
branch
from
54ffd4e to
3d1b4a1
Compare
fee1-dead
changed the title
|
@rustbot ready |
|
Sorry, I made another conflict by #1380 and #1381, could you also order like below? {
"rust-analyzer.checkOnSave.overrideCommand": [
"python3",
"x.py",
"check",
"--json-output"
],
"rust-analyzer.rustfmt.overrideCommand": [
"./build/$TARGET_TRIPLE/stage0/bin/rustfmt",
"--edition=2021"
],
"rust-analyzer.procMacro.enable": true,
"rust-analyzer.cargo.buildScripts.enable": true,
"rust-analyzer.cargo.buildScripts.overrideCommand": [
"./build/$TARGET_TRIPLE/stage0/bin/cargo",
"check",
"-p",
"rustc_driver",
"--message-format=json"
],
"rust-analyzer.cargo.useRustcWrapperForBuildScripts": true,
"rust-analyzer.rustc.source": "./Cargo.toml",
"rust-analyzer.linkedProjects": [
"Cargo.toml",
"src/bootstrap/Cargo.toml"
]
} |
|
@JohnTitor from what I see on R-A source code https://github.com/rust-lang/rust-analyzer/blob/master/crates/rust-analyzer/src/config.rs, |
Those configuration are there so we can suggest to people in libs to disable them later and would make the instructions clearer. |
fee1-dead
force-pushed
the
suggested-config
branch
from
3d1b4a1 to
266474a
Compare
| > the version `2022-04-14` this was fixed by introducing an option for customizing | ||
| > the command rust-analyzer uses to run build scripts. Please ensure that `rust-analyzer` | ||
| > is up to date before enabling these options. If you do not wish to enable | ||
| > `rust-analyzer.cargo.useRustcWrapperForBuildScripts`, the |
There was a problem hiding this comment.
Above the option is called rust-analyzer.cargo.buildScripts.useRustcWrapper, but here it is called by a different name?
There was a problem hiding this comment.
It got renamed, rust-analyzer.cargo.buildScripts.useRustcWrapper is the new name and rust-analyzer.cargo.useRustcWrapperForBuildScripts is the old name that would still work.
There was a problem hiding this comment.
I think this sentence should be left out anyway.
| @@ -38,8 +38,9 @@ you can write: <!-- date: 2022-04 --><!-- the date comment is for the edition be | |||
| ], | |||
| "rust-analyzer.procMacro.enable": true, | |||
| "rust-analyzer.cargo.buildScripts.enable": true, | |||
| "rust-analyzer.cargo.buildScripts.useRustcWrapper": true, | |||
There was a problem hiding this comment.
This seems to be set by default, why set it explicitly here?
(Same for the enable options.)
There was a problem hiding this comment.
I think it'd be better to not have these lines.
- If people have this explicitly globally disabled, we should respect that.
- For the libs suggestion, we can just have another code block saying "also add these things".
|
closing, as #1384 is better |
This has been fixed by rust-lang/rust-analyzer#11956, but I am not sure if this should be suggested for everyone - including those only working on the library - but with the rustc wrapper the performance impact is minimal.